Repository navigation
fix(store): guard findIndex(-1) before indexing in store actions - #3219
AndyScherzinger wants to merge 2 commits into
Conversation
|
Claude review, reduced to relevant findings Review: #3219: guard
|
0610b5a to
31aa558
Compare
|
Findings 1–3 are fixed in 31aa558 (amended into the original commit, force-pushed):
Finding 4 ( |
31aa558 to
0ae5a6d
Compare
Several store actions dereferenced or spliced a findIndex() result without guarding the -1 case: - favoriteView/removeFavoriteView/favoriteTable/removeFavoriteTable threw a TypeError at this.X[-1].favorite when toggling a favorite on a node absent from the local store. - removeView/removeTable/removeContext and the transfer actions called splice(index, 1) with index -1, which removes the LAST element instead of the intended (absent) one. - updateView/reloadViewsOfTable assigned to arr[-1], silently creating a bogus "-1" property instead of updating the view. - setTable spliced at -1, replacing the LAST table with the one it was asked to set; setView assigned to views[-1]. Both are reached by every caller, not only the favorite actions guarded above. - updateTable and updateContext assigned to tables[-1] and contexts[-1]. Return early (favorites) or skip the write, so a server success never corrupts the local store. An absent id means "nothing to update" rather than "insert": on a mutation echo it is missing because the node was removed while the request was in flight, and adding it back would resurrect it as a navigation entry that 404s on click. setContext is the exception and keeps its insert, which exists so that loading one application by its URL before the list is there stays visible, so updateContext guards at the call site instead of routing through it. updateView and reloadViewsOfTable now delegate to setView rather than repeating its body, and setTable assigns by index instead of splicing, as AGENTS.md requires for Vue 3. Assisted-by: ClaudeCode:claude-opus-4-8 Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
insertNewView looks the parent table up in this.tables and pushes the new view into it. The lookup is outside the try block, so when the table is not in the local store the push throws a TypeError and the action never returns the new view id. The table is absent whenever the view was created on a table the user only reaches through a view share: findAll returns the tables the user owns or has a table share on, so a directly shared view has no table entry. Cloning such a view then reports "Could not create new view" even though the server created it, leaving an orphan behind. Skip the push when the table is unknown; the view is already in this.views, which is what the navigation renders for shared views. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
0ae5a6d to
e8a82ca
Compare
Guards every
findIndex()result insrc/store/store.jsagainst the-1(not-found) case before indexing or splicing. When the affected node was not present in the local store, a successful server call could still corrupt local state:this.X[-1].favorite = …threw aTypeError(favorite toggles),splice(index, 1)withindex === -1removed the last element (remove/transfer view, table, context), andarr[index] = viewcreated a bogus"-1"property (updateView/reloadViewsOfTable). Each site now returns early, skips the splice, or pushes the missing item instead.Pre-existing on
main; split out of #2476 (personal archiving) per the focused-PR guideline so that PR stays scoped to archiving.src/store/data.jshas the same bug inupdateColumnandupdatePublicRowand is fixed in #3220.🖼️ Screenshots
n/a — no visual change (internal store guards only).
🏁 Checklist
/backport to stableX.X🤖 AI (if applicable)